Subject: fix for vulnerability CVE-2011-3342 for OpenTTD 1.0.0 - 1.0.4 (Buffer overflows in savegame loading)
From: OpenTTD developer team <info@openttd.org>
Origin: backport, https://github.com/OpenTTD/OpenTTD/commit/81074e0 https://github.com/OpenTTD/OpenTTD/commit/ef09794
Bug: https://github.com/OpenTTD/OpenTTD/issues/4748 https://github.com/OpenTTD/OpenTTD/issues/4717

In multiple places indices in savegames are not properly validated that allow 
(remote) attackers to cause a denial of service (crash) and possibly execute 
arbitrary code via unspecified vectors.

The bug is exploitable by passing someone a modified savegame, be it via a file
sharing site, or by running a server. In case of the server the user only has
to be able to login into the server, which is easy to accomplish: set no server
password and do not ban anyone. Then upon joining the server the savegame
will be downloaded and subsequently loaded.

Note that versions before 0.5.0 are vulnerable as well. However, these versions
are over five years old and not supported anymore. Therefore no patches for
earlier versions are provided. Before 0.3.5 it is not possible to exploit this
bug via the internet as multiplayer over internet did not exist yet.


Index: src/saveload/saveload.cpp
===================================================================
--- src/saveload/saveload.cpp	(revision 20804)
+++ src/saveload/saveload.cpp	(working copy)
@@ -203,6 +203,11 @@
 	throw std::exception();
 }
 
+void SlErrorCorrupt(const char *message)
+{
+	SlError(STR_GAME_SAVELOAD_ERROR_BROKEN_SAVEGAME, message);
+}
+
 typedef void (*AsyncSaveFinishProc)();
 static AsyncSaveFinishProc _async_save_finish = NULL;
 static ThreadObject *_save_thread;
Index: src/saveload/company_sl.cpp
===================================================================
--- src/saveload/company_sl.cpp	(revision 20804)
+++ src/saveload/company_sl.cpp	(working copy)
@@ -247,6 +247,7 @@
 	SlObject(&c->cur_economy, _company_economy_desc);
 
 	/* Write old economy entries. */
+	if (c->num_valid_stat_ent > lengthof(c->old_economy)) SlErrorCorrupt("Too many old economy entries");
 	for (i = 0; i < c->num_valid_stat_ent; i++) {
 		SlObject(&c->old_economy[i], _company_economy_desc);
 	}
Index: src/saveload/cheat_sl.cpp
===================================================================
--- src/saveload/cheat_sl.cpp	(revision 20804)
+++ src/saveload/cheat_sl.cpp	(working copy)
@@ -32,6 +32,8 @@
 {
 	Cheat *cht = (Cheat*)&_cheats;
 	size_t count = SlGetFieldLength() / 2;
+	/* Cannot use lengthof because _cheats is of type Cheats, not Cheat */
+	if (count > sizeof(_cheats) / sizeof(Cheat)) SlErrorCorrupt("Too many cheat values");
 
 	for (uint i = 0; i < count; i++) {
 		cht[i].been_used = (SlReadByte() != 0);
Index: src/saveload/strings_sl.cpp
===================================================================
--- src/saveload/strings_sl.cpp	(revision 20804)
+++ src/saveload/strings_sl.cpp	(working copy)
@@ -120,7 +120,12 @@
 	int index;
 
 	while ((index = SlIterateArray()) != -1) {
+		if (index >= 512) SlErrorCorrupt("Invalid old name index");
+		if (SlGetFieldLength() > 32) SlErrorCorrupt("Invalid old name length");
+
 		SlArray(&_old_name_array[32 * index], SlGetFieldLength(), SLE_UINT8);
+		/* Make sure the old name is null terminated */
+		_old_name_array[32 * index + 31] = '\0';
 	}
 }
 
Index: src/saveload/saveload.h
===================================================================
--- src/saveload/saveload.h	(revision 20804)
+++ src/saveload/saveload.h	(working copy)
@@ -50,6 +50,7 @@
 SaveOrLoadResult SaveOrLoad(const char *filename, int mode, Subdirectory sb, bool threaded = true);
 void WaitTillSaved();
 void DoExitSave();
+NORETURN void SlErrorCorrupt(const char *message);
 
 
 typedef void ChunkSaveLoadProc();
Index: src/saveload/ai_sl.cpp
===================================================================
--- src/saveload/ai_sl.cpp	(revision 20804)
+++ src/saveload/ai_sl.cpp	(working copy)
@@ -66,6 +66,8 @@
 
 	CompanyID index;
 	while ((index = (CompanyID)SlIterateArray()) != (CompanyID)-1) {
+		if (index >= MAX_COMPANIES) SlErrorCorrupt("Too many AI configs");
+
 		_ai_saveload_version = -1;
 		SlObject(NULL, _ai_company);
 

